Skip to content

fixes #439: Added styles for code-block#440

Open
rstefko wants to merge 11 commits into
owncloud:masterfrom
rstefko:master
Open

fixes #439: Added styles for code-block#440
rstefko wants to merge 11 commits into
owncloud:masterfrom
rstefko:master

Conversation

@rstefko

@rstefko rstefko commented Jan 21, 2023

Copy link
Copy Markdown

No description provided.

@sonarqubecloud

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

1 similar comment
@sonarqubecloud

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarqubecloud

sonarqubecloud Bot commented Jul 2, 2023

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarqubecloud

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Comment thread templates/note.php
@@ -1,4 +1,2 @@
<div id="app-navigation-toggle" class="icon-menu" style="display:none;"></div>
<pre editor notes-timeout-change="save()" notes-autofocus class="editor" ng-class="{saving: isSaving()}">{{note.content}}</pre>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change fixes #369. I'm very interested to see it merged @DeepDiver1975 @jnweiger

@dj4oC dj4oC left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Security: None found.
  • Stability: Title says "styles for code-block" but the diff is much broader: adds a notes-search feature (noteFilter.js, controller changes, new markup in templates/main.php), a PWA manifest.json, .gitignore entries for build artifacts, and — notably — removes the <div id="app-navigation-toggle"> wrapper (both its opening and closing tag) from templates/note.php. That structural DOM removal isn't explained by the PR's stated intent and could affect layout/nav-toggle behavior elsewhere in the app; it needs explicit justification and manual UI verification before merge.
  • Performance: N/A.
  • Test coverage: No tests added for the new search/filter logic (noteFilter.js, the debounced search handlers in notescontroller.js).
  • TODOs found: 0
  • Dependency touched: no
  • CI status: not re-verified (PR from 2023; previously reviewed with a comment only, no approval)

Verdict

Comment — please split this into separate PRs (search feature vs. code-block styling vs. the nav-toggle div removal) and confirm the DOM structure change is intentional and tested. Not blocking as no clear security/data-loss issue is visible, but scope and risk should be narrowed before merge.

🤖 Automated review by Claude Code (security · stability · performance · coverage)


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants